Skip to content

Fix #748 (Move the argument count check into Macro::getMacroParameters()) - #749

Merged
danmar merged 2 commits into
masterfrom
fix-748
Nov 9, 2026
Merged

danmar merged 2 commits into
masterfrom
fix-748

Conversation

@danmar

@danmar danmar commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Check the number of arguments where they are read instead of in the caller. Add a test for calls with the wrong number of arguments.

Check the number of arguments where they are read instead of in the
caller. Add a test for calls with the wrong number of arguments.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@dmcppcheck dmcppcheck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

I found no functional problems. The refactoring keeps the old behaviour: I compared the PR with base on a set of edge cases, and the output and diagnostics were identical every time. My only finding is that the new doc comment of getMacroParameters() is wrong in one case. For a function-like macro whose name is not followed by (, the function now throws wrongNumberOfParameters. The comment says it returns an empty vector. That happens when the name is the last token. The only caller hides this by returning early when the next token is not (, but anyone who reads the comment and calls the function will get the wrong idea.

Testing

  • make -j8 testrunner simplecpp CXXOPTS=-Werror in pr/ built cleanly.
  • ./testrunner: 0 failed, including the new define_wrong_number_of_arguments.
  • python3 run-tests.py: 176 tests, 0 failed, 15 todos. base gives the same numbers.
  • python3 -m pytest integration_test.py: 128 passed.
  • ./selfcheck.sh: exit code 0.

I built base/ and ran both simplecpp binaries on scratch files. Every case gave identical output and diagnostics in base and the PR:

  • a function-like macro name as the last token of the file (#define f(a) a\nf, also with a variadic macro)
  • a call whose arguments are completed outside the #define (#define g f(1 / g,2))
  • f x with parenthesised x inside another macro
  • arguments that span several lines
  • an unterminated f(1
  • __VA_OPT__ with empty variadic arguments
  • __COUNTER__ in arguments, with too few and with too many arguments

One limitation is not caused by this PR, since base behaves the same. A function-like macro name as the very last token, as in #define f(a) a\nf or ... f\n at the end of the file, gives "Wrong number of parameters". gcc -E simply outputs f.


Automated review of c764bc8 by Claude. The pull request was built and tested locally.

Comment thread simplecpp.cpp Resolved
@danmar
danmar merged commit 22522dd into master Nov 9, 2026
47 checks passed
@danmar
danmar deleted the fix-748 branch November 9, 2026 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants